Skip to content

Conversation

@kparzysz
Copy link
Contributor

Generate pre-lowering representations of several clauses for which frontend support has been added: ALIGN, AT, LOOPRANGE, MESSAGE, SEVERITY and USE.

Generate pre-lowering representations of several clauses for which
frontend support has been added: ALIGN, AT, LOOPRANGE, MESSAGE, SEVERITY
and USE.
@kparzysz kparzysz requested review from ergawy and tblah December 15, 2025 16:45
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Dec 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 15, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

Generate pre-lowering representations of several clauses for which frontend support has been added: ALIGN, AT, LOOPRANGE, MESSAGE, SEVERITY and USE.


Full diff: https://github.com/llvm/llvm-project/pull/172334.diff

1 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+31-11)
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 9ea4e8fcd6c0e..7ae5159c2e086 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -104,6 +104,7 @@ Object makeObject(const parser::Name &name,
 Object makeObject(const parser::Designator &dsg,
                   semantics::SemanticsContext &semaCtx) {
   evaluate::ExpressionAnalyzer ea{semaCtx};
+  auto restore{ea.AllowWholeAssumedSizeArray(true)};
   SymbolWithDesignator sd = getSymbolAndDesignator(ea.Analyze(dsg));
   SymbolAndDesignatorExtractor::verify(sd);
   return Object{std::get<0>(sd), std::move(std::get<1>(sd))};
@@ -112,6 +113,7 @@ Object makeObject(const parser::Designator &dsg,
 Object makeObject(const parser::StructureComponent &comp,
                   semantics::SemanticsContext &semaCtx) {
   evaluate::ExpressionAnalyzer ea{semaCtx};
+  auto restore{ea.AllowWholeAssumedSizeArray(true)};
   SymbolWithDesignator sd = getSymbolAndDesignator(ea.Analyze(comp));
   SymbolAndDesignatorExtractor::verify(sd);
   return Object{std::get<0>(sd), std::move(std::get<1>(sd))};
@@ -431,8 +433,8 @@ Affinity make(const parser::OmpClause::Affinity &inp,
 
 Align make(const parser::OmpClause::Align &inp,
            semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: align");
+  // inp.v -> OmpAlignClause
+  return Align{/*Alignment=*/makeExpr(inp.v.v, semaCtx)};
 }
 
 Aligned make(const parser::OmpClause::Aligned &inp,
@@ -486,8 +488,16 @@ Allocator make(const parser::OmpClause::Allocator &inp,
 
 At make(const parser::OmpClause::At &inp,
         semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: at");
+  // inp.v -> OmpAtClause
+  CLAUSET_ENUM_CONVERT( //
+      convertActionTime, parser::OmpAtClause::ActionTime, At::ActionTime,
+      // clang-format off
+      MS(Compilation, Compilation)
+      MS(Execution,   Execution)
+      // clang-format om
+  );
+
+  return At{/*ActionTime=*/convertActionTime(inp.v.v)};
 }
 
 // Never called, but needed for using "make" as a Clause visitor.
@@ -1089,7 +1099,10 @@ Link make(const parser::OmpClause::Link &inp,
 
 Looprange make(const parser::OmpClause::Looprange &inp,
                semantics::SemanticsContext &semaCtx) {
-  llvm_unreachable("Unimplemented: looprange");
+  // inp.v -> OmpLooprangeClause
+  auto &[begin, count]{inp.v.t};
+  return Looprange{
+      {/*Begin=*/makeExpr(begin, semaCtx), /*Count=*/makeExpr(count, semaCtx)}};
 }
 
 Map make(const parser::OmpClause::Map &inp,
@@ -1218,8 +1231,8 @@ Match make(const parser::OmpClause::Match &inp,
 
 Message make(const parser::OmpClause::Message &inp,
              semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: message");
+  // inp.v -> OmpMessageClause
+  return Message{/*MsgString=*/makeExpr(inp.v.v, semaCtx)};
 }
 
 Nocontext make(const parser::OmpClause::Nocontext &inp,
@@ -1471,8 +1484,15 @@ SelfMaps make(const parser::OmpClause::SelfMaps &inp,
 
 Severity make(const parser::OmpClause::Severity &inp,
               semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: severity");
+  // inp.v -> OmpSeverityClause
+  CLAUSET_ENUM_CONVERT( //
+      convertSevLevel, parser::OmpSeverityClause::SevLevel, Severity::SevLevel,
+      // clang-format off
+      MS(Fatal,   Fatal)
+      MS(Warning, Warning)
+      // clang-format om
+  );
+  return Severity{/*SevLevel=*/convertSevLevel(inp.v.v)};
 }
 
 Shared make(const parser::OmpClause::Shared &inp,
@@ -1622,8 +1642,8 @@ Update make(const parser::OmpClause::Update &inp,
 
 Use make(const parser::OmpClause::Use &inp,
          semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: use");
+  // inp.v -> OmpUseClause
+  return Use{/*InteropVar=*/makeObject(inp.v.v, semaCtx)};
 }
 
 UseDeviceAddr make(const parser::OmpClause::UseDeviceAddr &inp,

Copy link
Member

@ergawy ergawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Krzysztof, no objections to code changes. Can we add some tests here somehow? At least, I think we should add regression tests for the 2 makeObject oveloads updated by the PR.

@kparzysz
Copy link
Contributor Author

Some background:
The parser and the semantic analysis + lowering use different representation of expressions. AST uses parser::Expr, the rest uses evaluate::Expr. The conversion from AST to the "evaluate" form is done by ExpressionAnalyzer. One problem with the analyzer is that it can diagnose (and issue diagnostic messages) on its own. One example of that is that it can complain about whole assumed-size arrays used in an expression. This is bad because we want to decide when to issue a message, and some such cases are legitimate. Thankfully, there is a mechanism to suppress that (via the "AllowWholeAssumedSizeArray" function), and that's what I used here.

The background here is that I'm working on doing this clause conversion very early. It willl be used to decompose compound directives and it will help rewrite the AST (and happen before any semantic checks). Currently, the semantic checks reject all cases that would trigger the assumed-size array issue in the analyzer before we get to lowering, so the makeObject functions are never called with an assumed-size array. This is not completely correct though, there are two clauses where we should accept them in OpenMP 6.0. I have a fix for that here: #172510.

With the fix the makeObject function are called with an actual whole assumed-size array. From the point of view of the user there is no visible effect though (i.e. no messages), but that's only because at this point we no longer print any semantic messages. The message is still generated (by the analyzer), but we are past the point where we call the function that prints all the semantic messages. The change from this PR is to stop the message from being generated in the first place.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra background. I can see why this would be inconvenient to test currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants